Skip to content

[dicom_archive/imaging_uploader] Residual fixes for advanced permissions #9761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ridz1208
Copy link
Collaborator

Brief summary of changes

  • Have you updated related documentation?

Testing instructions (if applicable)

Link(s) to related issue(s)

  • Resolves # (Reference the issue this fixes, if any.)

@github-actions github-actions bot added Language: SQL PR or issue that update SQL code Language: PHP PR or issue that update PHP code Module: imaging_uploader PR or issue related to imaging_uploader module Module: dicom_archive PR or issue related to dicom_archive module labels Apr 17, 2025
@ridz1208 ridz1208 added the Critical to release PR or issue is key for the release to which it has been assigned label May 13, 2025
@ridz1208
Copy link
Collaborator Author

@nicolasbrossard the issue was just static, feel free to review and test when you get a chance

if ($advancedperms === 'true') {
// If config setting is enabled, check the user's sites and projects
// site/project match + user's own uploads
$where = $where . " AND s.ProjectID IN ('$projectString')";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work if you put single quotes around $projectString. Why not use

$where = $where . " AND FIND_IN_SET(s.ProjectID, :projectString)";

and bind $projectString to projectString when calling pselectRow?

(same thing for $centerString)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolasbrossard just curious why it doesnt work? the single quotes will match the ones in the JOIN function for centerstring and projectstring.

its also a copy of the code in here https://github.com/aces/Loris/blob/main/modules/imaging_uploader/php/imaging_uploader.class.inc#L100

I dont mind FIND_IN_SET() I just hadn't used it in a while, so if this is problematic I'll switch to it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you are right! I had not noticed that $projectString was set to implode("','", $user->getProjectIDs());. I had not noticed that each project ID was surrounded with single quote so your solution will work. All good then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Critical to release PR or issue is key for the release to which it has been assigned Language: PHP PR or issue that update PHP code Language: SQL PR or issue that update SQL code Module: dicom_archive PR or issue related to dicom_archive module Module: imaging_uploader PR or issue related to imaging_uploader module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants